Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 52-73: The debounce currently catches all errors so the
success-path updateDocument call clears isDirty even when save fails; remove the
empty catch here (or rethrow the error) so failures from saveContent(page.id,
newContent) propagate and prevent updating isDirty. Update the setContent
callback (inside the setTimeout) to either await saveContent without a try/catch
or, if you must catch to log, rethrow the caught error; ensure saveContent still
throws on failure so useDocumentManagerStore.getState().updateDocument(page.id,
{ isDirty: false, lastSaved: ... }) only runs when saveContent resolves
successfully.
- Around line 88-92: The mounted effect calls
useDocumentManagerStore.getState().createDocument(page.id, ...) but
createDocument is a no-op if the doc exists, causing stale content when
revisiting; fix by first reading the cached document via
useDocumentManagerStore.getState().getDocument(page.id) (or equivalent getter)
and if it exists compare its text to the fresh page.content and call an update
method (e.g., updateDocument or createDocument with overwrite) when they differ;
additionally add a cleanup in the same useEffect that removes or clears the
document on unmount (e.g.,
useDocumentManagerStore.getState().deleteDocument(page.id) or resetDocument) so
documents don't persist across remounts when out-of-band server changes occur.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 112-124: The cleanup effect captures a stale page.id; update the
component to track the current page id in a ref (e.g., pageIdRef) and keep it in
sync whenever props.page.id changes, then use pageIdRef.current inside the
useEffect cleanup instead of the closed-over page.id; specifically, create
pageIdRef (like saveContentRef), set pageIdRef.current = page.id in a short
effect that runs on page.id changes, and in the existing useEffect cleanup
replace references to page.id with pageIdRef.current when calling
saveContentRef.current(...) and
useDocumentManagerStore.getState().clearDocument(...).
- Around line 23-32: CanvasPageView never registers editing with the global
editing store; add a useEffect inside the CanvasPageView component that calls
useEditingStore.getState().startEditing() when the component enters edit mode
(e.g., when activeTab === 'edit' or when the editor is focused) and returns a
cleanup that calls useEditingStore.getState().stopEditing() (or the appropriate
end method) to unregister on unmount or when leaving edit mode; place this
effect in CanvasPageView near the other refs/hooks (alongside
containerRef/saveTimeoutRef) so SWR revalidation is suppressed while editing.

---

Nitpick comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 246-251: The memo comparator on CanvasPageView only prevents
renders when page.id and page.content are unchanged, but because
CanvasPageView's empty-deps unmount effect (the cleanup run on component
unmount) currently closes over the original page.id, update that effect to read
the current page id from a ref instead of the stale prop: add a pageIdRef (e.g.,
const pageIdRef = useRef(page.id)), update pageIdRef.current on each render or
in a useEffect that depends on page.id, and then reference pageIdRef.current
inside the cleanup callback so cleanup uses the latest page id when the
component unmounts; keep the React.memo comparator as-is (export default
React.memo(CanvasPageView, ...)).

In `@apps/web/src/hooks/__tests__/useDevices.test.ts`:
- Line 75: Add a guard asserting that swrConfig.onSuccess is defined before
invoking it in the test: replace the direct non-null assertion call with an
assertion like expect(swrConfig.onSuccess).toBeDefined(); followed by the call
to swrConfig.onSuccess!(); Apply this change for both places where
swrConfig.onSuccess is invoked in the useDevices.test.ts tests so the test fails
with a clear assertion if the handler is removed.

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 140-173: The socket handler handleContentUpdate must use an
AbortController so stale fetches don't overwrite newly-initialized documents:
inside the useEffect create a controller-per-invocation (e.g., create a new
AbortController at the top of handleContentUpdate or push it into an array
controllers declared in the effect scope), pass controller.signal into
fetchWithAuth, and on cleanup for the effect abort all active controllers and
remove the socket listener via socket.off('page:content-updated',
handleContentUpdate); also make the catch ignore AbortError (or skip calling
updateContentFromServer if the fetch was aborted) to avoid applying stale
content; reference handleContentUpdate, fetchWithAuth, updateContentFromServer,
and the socket.on/off setup when making the change.

---

Nitpick comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 82-93: The updateContentFromServer handler currently clears the
debounce timer but does not increment the in-flight save version, so an
already-running save (awaiting saveContent) can still pass the
saveVersionRef.current === version check and overwrite server-updated state;
inside updateContentFromServer (function name) after clearing saveTimeoutRef
(saveTimeoutRef.current) and before calling updateDocument, increment or bump
saveVersionRef.current (e.g., ++saveVersionRef.current or assign a new token) so
any in-flight save will fail its version guard (saveVersionRef and the
saveContent version check) and be prevented from writing stale
lastSaved/lastUpdateTime.
- Around line 259-264: The custom React.memo comparator on CanvasPageView
currently only compares prevProps.page.id and prevProps.page.content which will
silently prevent re-renders if new TreePage fields are later used; update the
memo call by adding a concise comment above the comparator explaining that the
comparator intentionally limits checks to page.id and page.content (and that any
time a new page field like page.title or page.permissions is consumed by
CanvasPageView the comparator must be updated to include it or be removed), and
ensure the comment references CanvasPageView, prevProps/nextProps and the
page.id/page.content checks so future contributors understand why only those
fields are compared.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/CanvasPageView.save.test.ts`:
- Around line 52-53: The test file uses the React namespace in the ErrorBoundary
prop type (ErrorBoundary: ({ children }: { children: React.ReactNode }) =>
children) but never imports React; add a proper React type import and update the
type usage accordingly — e.g., import type { ReactNode } from 'react' and change
the prop annotation to ({ children }: { children: ReactNode }) or alternatively
import React and keep React.ReactNode; update the ErrorBoundary declaration to
use the imported type.

In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 134-137: The post-save .then() unconditionally calls
useDocumentManagerStore.getState().clearDocument(id), which can clear a freshly
remounted document if the component remounts before the save resolves; change
the callback to guard against remount races by verifying the saved id still
represents the same live document before clearing—e.g., capture a local mount
token or check a current active document id/version from useDocumentManagerStore
(or an isMounted flag) and only call clearDocument(id) when the token/active id
matches the original id; target saveContentRef.current(...)'s .then() handler
and replace the unconditional clearDocument call with this guarded check.

In
`@apps/web/src/components/layout/middle-content/page-views/sheet/SheetView.tsx`:
- Around line 1550-1553: The fetchWithAuth call in SheetView.tsx constructs a
query string with user.id unencoded which can break URLs if the ID contains
reserved characters; update the call that builds
`/api/pages/${page.id}/permissions/check?userId=${user.id}` to encode the user
id (e.g., use encodeURIComponent(user.id)) before interpolation so the query
parameter is safely encoded while keeping the same abortController.signal and
existing fetchWithAuth usage.

---

Nitpick comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/canvas-save-lifecycle.test.ts`:
- Around line 1-165: The test file defines a save-lifecycle suite that overlaps
with CanvasPageView.save.test.ts; remove duplication by consolidating behaviors
into a single canonical test file: either delete this file
(apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/canvas-save-lifecycle.test.ts)
or merge any unique specs here into CanvasPageView.save.test.ts, ensuring you
keep the authoritative tests for debounce/version-guard/error/unmount scenarios
(tests referencing useDocumentManagerStore, createDocument, updateDocument,
clearDocument) and remove the redundant suite, update any test imports or CI
references, and run the test suite to confirm no regressions.

In
`@apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/CanvasPageView.save.test.ts`:
- Around line 79-281: Tests are mutating useDocumentManagerStore and calling
mockPatch directly so they never exercise CanvasPageView's debounce/unmount save
effects; refactor by keeping 1–2 pure store unit tests but move lifecycle
assertions into component-level tests that render CanvasPageView, simulate user
edits by invoking the MonacoEditor onChange handler (or dispatching events that
trigger CanvasPageView's update flow), advance fake timers to flush the
debounce, unmount the component to trigger the force-save effect, and then
assert store state and mockPatch calls to verify real save/version-guard/error
behaviors.

In `@apps/web/src/hooks/page-agents/__tests__/usePageAgents.test.ts`:
- Around line 53-55: The tests repeatedly extract the SWR mock call and its
config using the same pattern (vi.mocked(useSWR).mock.calls[...] and casting to
the config shape); create a small helper function (e.g., getMockSWRConfig or
extractSWRConfig) used by tests in usePageAgents.test.ts to accept the call
index (or default to 0) and return the typed config object so you can replace
repeated lines that compute swrCall and swrConfig (references: useSWR, swrCall,
swrConfig) with a single call to the helper to reduce duplication and improve
maintainability.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx`:
- Around line 25-27: The initial render falls back to an empty string causing a
flash of blank content; update the content fallback in the component to use the
page's server-provided content instead of '' — i.e., where you derive content
from useDocumentManagerStore (variable documentState and constant content),
change the fallback from '' to page.content so first paint shows initial
content; also sweep the other occurrences mentioned (the similar logic around
lines 98-111) and apply the same change to any other uses that default to ''
when documentState is not yet populated.
- Around line 66-76: saveContent calls from CanvasPageView currently omit
expectedRevision, allowing stale PATCHes to overwrite newer edits; either (A)
add revision tracking to the document store (e.g., store currentRevision on the
page state in useDocumentManagerStore and keep it updated when server responses
arrive) and change saveContent calls in CanvasPageView (and the unmount cleanup
path where saveTimeoutRef/ saveVersionRef are used) to include expectedRevision
when PATCHing, or (B) enforce single in-flight save by wiring an AbortController
per-page: store the controller on save start, pass its signal into saveContent,
abort any prior controller before starting a new save, and also abort pending
saves on unmount so stale requests cannot complete. Ensure references to
saveContent, saveTimeoutRef, saveVersionRef, and
useDocumentManagerStore.updateDocument are updated accordingly.

---

Outside diff comments:
In
`@apps/web/src/components/layout/middle-content/page-views/document/DocumentView.tsx`:
- Around line 131-152: In checkPermissions, fail closed by setting
isReadOnly(true) immediately before making the fetch and ensure any non-OK or
error paths (except AbortError) leave isReadOnly true; then only call
setIsReadOnly(!permissions.canEdit) after a successful response.ok and JSON
parse. Update the fetchWithAuth handling in checkPermissions to set
isReadOnly(true) prior to the request, set isReadOnly(true) when response.ok is
false, and in the catch block (except when error.name === 'AbortError') also set
isReadOnly(true) before logging the error so the previous page’s writable state
cannot leak.